Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Action Menu Button Example Using aria-activedescendant Test Plan to V2 Test Format #1092

Merged
merged 22 commits into from
Aug 8, 2024

Conversation

IsaDC
Copy link
Contributor

@IsaDC IsaDC commented Jul 31, 2024

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IsaDC

Overall, looks great. There are just a few details to address.

Since the tests include multiple elements that have names that need to be conveyed , please use the assertion format that includes the element being named. For example, instead of:

Name 'Actions' is conveyed

I recommend the format:

Name of the menu button, 'Actions', is conveyed

Also, please fill in the refId column in assertions.csv.

I believe JAWS and NVDA should have assertions for switching to interaction mode in:

  • In the first two tests for Tab and Shift+Tab
  • In Test 4: Open a menu for space and enter when in reading mode

@IsaDC IsaDC requested a review from mcking65 August 2, 2024 18:57
@IsaDC
Copy link
Contributor Author

IsaDC commented Aug 2, 2024

@mcking65 I have implemented the requested changes, except for adding the assertions for switching to interaction mode in Test 4: "Open a menu with space and enter when in reading mode," as these assertions already exist.
Since I am still getting familiar with the references, please feel free to correct any mistakes I may have made.
Thank you.

@mcking65
Copy link
Contributor

mcking65 commented Aug 6, 2024

@IsaDC

Thank you for the updates! I made a few editorial changes to assertions and updated references.

I have two more suggestions for "Test 6: Request information about a menu item".

  • Assert that insert+tab in JAWS and NVDA and ctrl-opt-f3 give the name and role of the menu
  • Test it in vc and browse mode as well

@IsaDC
Copy link
Contributor Author

IsaDC commented Aug 7, 2024

@mcking65 I implemented the requested changes

@mcking65
Copy link
Contributor

mcking65 commented Aug 8, 2024

@IsaDC

I found the need for some more changes. To ensure we could start testing tomorrow, I made the changes and merged.

the changes are listed below. I don't think they are contraversial; they are all consistent with prior plans. Please review the changes. If there are problems, open a new PR.

This list could be helpful when working on the other two menu button test plan refactors.

Modified the HTML so the 'afterlink' is immediately after the menu button. Previously it was after the text field so that pressing up arrow when navigating back did not go to the menu button.

Changed one of the name assertions used in tests 4, 5, and 6 for opening a menu. I added a new priority 3 assertion 'nameMenuActions' for asserting the name of the menu.

Previously, when tests 4,5, and 6 had focus on the menu item, they used the assertion 'nameActions', which is:

Name of the menu button, 'Actions', is conveyed

With the new assertion, those tests now have the assertion:

Name of the menu, 'Actions', is conveyed

I changed the priority of assertions related to posinset and setsize to 2; it was set at 1.

I changed the priority of assertion 'roleMenu' to 3; it was previously 1.

In the assertions column of tests.csv, I revised the order of assertions to be consistently name, role, posinset, setsize, optional assertions, mode switch. This puts the must assertions first in every test, followed by shoulds, then by may.

Added the mode switch assertion to the assertions column in tests.csv for tests 1 and 2. It is present in the commands files for those tests, but for the first 2 tests, missing from the assertions list.

Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @IsaDC, excellent work on this. It's now ready to ship.

@mcking65 mcking65 merged commit 3d7a119 into master Aug 8, 2024
7 checks passed
@mcking65 mcking65 deleted the tests/menu-button-actions-active-descendant-refactor branch August 8, 2024 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants